fix(cloudflare): Wait for async events to finish #18334
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
packages/cloudflare/src/client.ts
Outdated
| await spanCompletionRace; | ||
| } | ||
|
|
||
| this._resetSpanCompletionPromise(); |
There was a problem hiding this comment.
Bug: Race condition clears pending spans during flush
The unconditional call to _resetSpanCompletionPromise() after waiting for pending spans creates a race condition. If new spans start after the await completes but before the reset executes, their tracking state gets cleared. This causes those spans to not be waited for in subsequent flushes and potentially not be sent to Sentry. The reset should only clear the state that was waited for, not any new spans that started concurrently.
Member
Author
There was a problem hiding this comment.
That is actually a very good point. I actually added it for 1 reason, but by then the code looked a little different. The reason was that it could happen that there is some stale data somehow, but this couldn't happen anymore since there is a Promise.race and a conditional before.
Will remove it.
JPeer264
force-pushed
the
jp/wait-for-flush
branch
from
1dbcd6e to
4366338
Compare
JPeer264
force-pushed
the
jp/wait-for-flush
branch
from
4366338 to
3ef53d7
Compare
packages/cloudflare/src/client.ts
Outdated
| } | ||
|
|
||
| this._resetSpanCompletionPromise(); | ||
|
|
There was a problem hiding this comment.
Bug: Timeout budget not properly managed across operations
The timeout parameter represents the total time budget for the entire flush operation, but the implementation uses it twice: first for waiting for pending spans (line 84) and then passes the same value to super.flush(timeout) (line 101). This means the actual total time could be up to double the intended timeout. The remaining time budget should be calculated after the span wait completes and passed to the parent flush method.
3 tasks
andreiborza
approved these changes
Dec 2, 2025
| // Track span lifecycle to know when to flush | ||
| this.on('spanStart', span => { | ||
| const spanId = span.spanContext().spanId; | ||
| DEBUG_BUILD && debug.log('[CloudflareClient] Span started:', spanId); |
Member
There was a problem hiding this comment.
l/m: Does this add anything for users? I thin span logs already exist right?
|
|
||
| this.on('spanEnd', span => { | ||
| const spanId = span.spanContext().spanId; | ||
| DEBUG_BUILD && debug.log('[CloudflareClient] Span ended:', spanId); |
Member
There was a problem hiding this comment.
l/m: Same here, I think these aren't that useful.
|
|
||
| // If no more pending spans, resolve the completion promise | ||
| if (this._pendingSpans.size === 0 && this._resolveSpanCompletion) { | ||
| DEBUG_BUILD && debug.log('[CloudflareClient] All spans completed, resolving promise'); |
Member
Author
|
We discussed this offline. We leave the debug logs for now. They can be removed anytime if they seem not useful. |
dcramer
pushed a commit
to getsentry/sentry-mcp
that referenced
this pull request
Dec 2, 2025
In #652 we had to manually add the flush, which should actually be done by the Cloudflare SDK. This has now landed in `10.28.0` (via [this PR](getsentry/sentry-javascript#18334)) and no manual flushing has to be done anymore. Just to be sure sure I verified on top if everything has been sent locally. This is a trace from my local deployment: https://sentry-sdks.sentry.io/explore/traces/trace/434f1b1e8655444c875bce5c66645376/?fov=0%2C291&node=span-84a7a24ae593ee49&project=4510380305022976&source=traces&statsPeriod=15m&targetId=a505468e10176600×tamp=1764689933 I recommend to review this PR [without whitespaces enabled](https://github.com/getsentry/sentry-mcp/pull/663/files?w=1).
Doridian
pushed a commit
to foxCaves/foxCaves
that referenced
this pull request
Dec 3, 2025
This PR contains the following updates: | Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---| | [@sentry/react](https://github.com/getsentry/sentry-javascript/tree/master/packages/react) ([source](https://github.com/getsentry/sentry-javascript)) | [`10.27.0` -> `10.28.0`](https://renovatebot.com/diffs/npm/@sentry%2freact/10.27.0/10.28.0) |  |  | --- ### Release Notes <details> <summary>getsentry/sentry-javascript (@​sentry/react)</summary> ### [`v10.28.0`](https://github.com/getsentry/sentry-javascript/releases/tag/10.28.0) [Compare Source](getsentry/sentry-javascript@10.27.0...10.28.0) ##### Important Changes - **feat(core): Make `matcher` parameter optional in `makeMultiplexedTransport` ([#​10798](getsentry/sentry-javascript#10798 The `matcher` parameter in `makeMultiplexedTransport` is now optional with a sensible default. This makes it much easier to use the multiplexed transport for sending events to multiple DSNs based on runtime configuration. **Before:** ```javascript import { makeFetchTransport, makeMultiplexedTransport } from '@​sentry/browser'; const EXTRA_KEY = 'ROUTE_TO'; const transport = makeMultiplexedTransport(makeFetchTransport, args => { const event = args.getEvent(); if (event?.extra?.[EXTRA_KEY] && Array.isArray(event.extra[EXTRA_KEY])) { return event.extra[EXTRA_KEY]; } return []; }); Sentry.init({ transport, // ... other options }); // Capture events with routing info Sentry.captureException(error, { extra: { [EXTRA_KEY]: [ { dsn: 'https://key1@​sentry.io/project1', release: 'v1.0.0' }, { dsn: 'https://key2@​sentry.io/project2' }, ], }, }); ``` **After:** ```javascript import { makeFetchTransport, makeMultiplexedTransport, MULTIPLEXED_TRANSPORT_EXTRA_KEY } from '@​sentry/browser'; // Just pass the transport generator - the default matcher handles the rest! Sentry.init({ transport: makeMultiplexedTransport(makeFetchTransport), // ... other options }); // Capture events with routing info using the exported constant Sentry.captureException(error, { extra: { [MULTIPLEXED_TRANSPORT_EXTRA_KEY]: [ { dsn: 'https://key1@​sentry.io/project1', release: 'v1.0.0' }, { dsn: 'https://key2@​sentry.io/project2' }, ], }, }); ``` The default matcher looks for routing information in `event.extra[MULTIPLEXED_TRANSPORT_EXTRA_KEY]`. You can still provide a custom matcher function for advanced use cases. - **feat(nextjs): Support cacheComponents on turbopack ([#​18304](getsentry/sentry-javascript#18304 This release adds support for `cacheComponents` on turbopack builds. We are working on adding support for this feature in webpack builds as well. ##### Other Changes - feat: Publish AWS Lambda Layer for Node 24 ([#​18327](getsentry/sentry-javascript#18327)) - feat(browser): Expose langchain instrumentation ([#​18342](getsentry/sentry-javascript#18342)) - feat(browser): Expose langgraph instrumentation ([#​18345](getsentry/sentry-javascript#18345)) - feat(cloudflare): Allow specifying a custom fetch in Cloudflare transport options ([#​18335](getsentry/sentry-javascript#18335)) - feat(core): Add `isolateTrace` option to `Sentry.withMonitor()` ([#​18079](getsentry/sentry-javascript#18079)) - feat(deps): bump [@​sentry/webpack-plugin](https://github.com/sentry/webpack-plugin) from 4.3.0 to 4.6.1 ([#​18272](getsentry/sentry-javascript#18272)) - feat(nextjs): Add cloudflare `waitUntil` detection ([#​18336](getsentry/sentry-javascript#18336)) - feat(node): Add LangChain v1 support ([#​18306](getsentry/sentry-javascript#18306)) - feat(remix): Add parameterized transaction naming for routes ([#​17951](getsentry/sentry-javascript#17951)) - fix(cloudflare): Keep http root span alive until streaming responses are consumed ([#​18087](getsentry/sentry-javascript#18087)) - fix(cloudflare): Wait for async events to finish ([#​18334](getsentry/sentry-javascript#18334)) - fix(core): `continueTrace` doesn't propagate given trace ID if active span exists ([#​18328](getsentry/sentry-javascript#18328)) - fix(node-core): Handle custom scope in log messages without parameters ([#​18322](getsentry/sentry-javascript#18322)) - fix(opentelemetry): Ensure Sentry spans don't leak when tracing is disabled ([#​18337](getsentry/sentry-javascript#18337)) - fix(react-router): Use underscores in trace origin values ([#​18351](getsentry/sentry-javascript#18351)) - chore(tanstackstart-react): Export custom inits from tanstackstart-react ([#​18369](getsentry/sentry-javascript#18369)) - chore(tanstackstart-react)!: Remove empty placeholder implementations ([#​18338](getsentry/sentry-javascript#18338)) <details> <summary><strong>Internal Changes</strong></summary> - chore: Allow URLs as issue ([#​18372](getsentry/sentry-javascript#18372)) - chore(changelog): Add entry for [#​18304](getsentry/sentry-javascript#18304) ([#​18329](getsentry/sentry-javascript#18329)) - chore(ci): Add action to track all PRs as issues ([#​18363](getsentry/sentry-javascript#18363)) - chore(github): Adjust `BUGBOT.md` rules to flag invalid op and origin values during review ([#​18352](getsentry/sentry-javascript#18352)) - ci: Add action to create issue on gitflow merge conflicts ([#​18319](getsentry/sentry-javascript#18319)) - ci(deps): bump actions/checkout from 5 to 6 ([#​18268](getsentry/sentry-javascript#18268)) - ci(deps): bump peter-evans/create-pull-request from 7.0.8 to 7.0.9 ([#​18361](getsentry/sentry-javascript#18361)) - test(cloudflare): Add typechecks for cloudflare-worker e2e test ([#​18321](getsentry/sentry-javascript#18321)) </details> #### Bundle size 📦 | Path | Size | | ----------------------------------------------------------------------------------------------------- | --------- | | [@​sentry/browser](https://github.com/sentry/browser) | 24.22 KB | | [@​sentry/browser](https://github.com/sentry/browser) - with treeshaking flags | 22.76 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing) | 40.57 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Profiling) | 45.05 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay) | 78.08 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay) - with treeshaking flags | 68.05 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay with Canvas) | 82.65 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay, Feedback) | 94.61 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Feedback) | 40.51 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. sendFeedback) | 28.8 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. FeedbackAsync) | 33.66 KB | | [@​sentry/react](https://github.com/sentry/react) | 25.9 KB | | [@​sentry/react](https://github.com/sentry/react) (incl. Tracing) | 42.72 KB | | [@​sentry/vue](https://github.com/sentry/vue) | 28.56 KB | | [@​sentry/vue](https://github.com/sentry/vue) (incl. Tracing) | 42.32 KB | | [@​sentry/svelte](https://github.com/sentry/svelte) | 24.24 KB | | CDN Bundle | 26.57 KB | | CDN Bundle (incl. Tracing) | 41.22 KB | | CDN Bundle (incl. Tracing, Replay) | 76.9 KB | | CDN Bundle (incl. Tracing, Replay, Feedback) | 82.23 KB | | CDN Bundle - uncompressed | 78.09 KB | | CDN Bundle (incl. Tracing) - uncompressed | 122.4 KB | | CDN Bundle (incl. Tracing, Replay) - uncompressed | 235.71 KB | | CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed | 248.17 KB | | [@​sentry/nextjs](https://github.com/sentry/nextjs) (client) | 44.88 KB | | [@​sentry/sveltekit](https://github.com/sentry/sveltekit) (client) | 40.92 KB | | [@​sentry/node-core](https://github.com/sentry/node-core) | 50.06 KB | | [@​sentry/node](https://github.com/sentry/node) | 155.7 KB | | [@​sentry/node](https://github.com/sentry/node) - without tracing | 90.67 KB | | [@​sentry/aws-serverless](https://github.com/sentry/aws-serverless) | 105.61 KB | </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi4yNy4xIiwidXBkYXRlZEluVmVyIjoiNDIuMjcuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOltdfQ==--> Reviewed-on: https://git.foxden.network/foxCaves/foxCaves/pulls/12 Co-authored-by: Renovate <renovate@foxden.network> Co-committed-by: Renovate <renovate@foxden.network>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
(closes #18360)
Problem
In getsentry/sentry-mcp#652 we had to manually wait for the flush to ensure that async operations completed before the response was sent.
This was caused by the MCP server streaming data after the actual request has been finished. When using the Cloudflare SDK with MCP servers (or other streaming APIs), spans created during the streaming phase were not being flushed properly because the
flush()method would complete before these async spans finished.Solution
During the request, when a span is created for the streaming call, we now register listeners on the CloudflareClient for
spanStartandspanEndevents. The client tracks all pending spans and waits for them to complete before flushing.Specifically:
_pendingSpansSet to track active span IDs_spanCompletionPromise) to wait for all spans to completeflush()method to wait for pending spans before callingsuper.flush()Test
Added a comprehensive E2E test application (
cloudflare-mcp) that:The test confirms that spans are now flushed correctly without manual intervention.